Standardize loggings in the tools #205
Conversation
There was a problem hiding this comment.
Pull request overview
This PR standardizes logging across the codebase by replacing ad-hoc print statements with Python's built-in logging module and centralizing the logging configuration in concore.py.
Changes:
- Centralized logging configuration in concore.py with DEBUG level, timestamps, and logger names
- Converted print statements to appropriate logging calls (info, debug, error) across 11 tool files
- Added library-specific logging level suppression for matplotlib, PIL, urllib3, and requests
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| concore.py | Centralized logging configuration with DEBUG level, timestamps, and muted noisy library loggers |
| tools/shannon.py | Converted print statements to logging.info and logging.debug |
| tools/pwrap.py | Converted print statements to logging calls at appropriate levels (info/debug/error) |
| tools/plotymlag.py | Converted print statements to logging.info and logging.debug |
| tools/plotym.py | Converted print statements to logging.info and logging.debug |
| tools/plotu.py | Converted print statements to logging.info |
| tools/pidsig.py | Converted print statements to logging.info and logging.error |
| tools/pidmayuresh.py | Converted print statements to logging calls at appropriate levels |
| tools/pid2.py | Converted print statements to logging.info and logging.debug |
| tools/learn.py | Converted print statement to logging.info |
| tools/cwrap.py | Converted print statements to logging calls at appropriate levels (info/debug/error) |
| tools/cardiac_pm.py | Added startup message and converted print to logging.debug |
| tools/bangbang.py | Added startup message and converted print to logging.debug |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tools/pwrap.py
Outdated
| print(yuyu) | ||
| print(name1+'='+init_simtime_u) | ||
| print(name2+'='+init_simtime_ym) | ||
| logging.info(f"API Key: {apikey}") |
There was a problem hiding this comment.
Security concern: logging API keys at INFO level could expose sensitive credentials in production logs. Consider either removing this log statement, masking the API key (e.g., showing only the first/last few characters), or changing to DEBUG level so it can be easily disabled in production environments.
tools/cwrap.py
Outdated
| print(yuyu) | ||
| print(name1+'='+init_simtime_u) | ||
| print(name2+'='+init_simtime_ym) | ||
| logging.info(f"API Key: {apikey}") |
There was a problem hiding this comment.
Security concern: logging API keys at INFO level could expose sensitive credentials in production logs. Consider either removing this log statement, masking the API key (e.g., showing only the first/last few characters), or changing to DEBUG level so it can be easily disabled in production environments.
There was a problem hiding this comment.
@GREENRAT-K405 yes, pls make this level DEBUG. I don't think it is necessary to be in the INFO level.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@pradeeban, please review the changes. |
|
@pradeeban, I have done the changes |
2 participants
This standardizes the output mechanism by replacing ad-hoc print statements with python's built-in logging module.
I have also, changes the centralized logging configuration in concore.py to ensure consistent formatting (timestamps and the log level.
Benefits:
here is the run of demo/sample3:
this closes #192